-
-
Notifications
You must be signed in to change notification settings - Fork 681
feat: Add no-empty-component-block rule #1222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add no-empty-component-block rule #1222
Conversation
create(context) { | ||
return { | ||
Program(node) { | ||
const componentBlocks = node.templateBody.parent.children |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since vue-eslint-parser
v7.0.0+ you can use parserServices.getDocumentFragment()
.
This allows you to get the root element without the <template>
block.
https://github.com/mysticatea/vue-eslint-parser/releases/tag/v7.0.0
I'm using this API in the padding-line-between-blocks
rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool feature👍
Thanks for sharing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR!
I have some requests.
] | ||
}, | ||
{ | ||
code: '<template></template><script></script><style></style>', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add testcases that contains whitespaces such as line breaks? It probably won't report because it contains VText.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow. Good point!
I'll fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some invalid cases.
fbbb360
to
4e94cdf
Compare
return ( | ||
componentBlock.children.length === 1 && | ||
componentBlock.children[0].type === 'VText' && | ||
/^(\s|\n)+$/.test(componentBlock.children[0].value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this regex is cover all cases...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe using /^\s*$/
will also work.
However, I'm not good at using regular expressions, so I often do the following.
!componentBlock.children[0].value.trim()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems nice.
I reflected this code.
componentBlock.name !== 'script' && | ||
componentBlock.name !== 'style' | ||
) | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed. Use continue
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
return | ||
|
||
// https://vue-loader.vuejs.org/spec.html#src-imports | ||
if (hasAttributeSrc(componentBlock)) return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use continue
here as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thank you!
close #952
This PR adds vue/no-empty-component-block rule.